Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[do not merge] Revert "Convert SerializedDepGraph to be a struct-of-arrays" #57070

Closed
wants to merge 1 commit into from
Closed

[do not merge] Revert "Convert SerializedDepGraph to be a struct-of-arrays" #57070

wants to merge 1 commit into from

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Dec 23, 2018

Let's check if #49069 was actually helpful.

r? @michaelwoerister

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 23, 2018
@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 23, 2018

@bors try

@bors
Copy link
Contributor

bors commented Dec 23, 2018

⌛ Trying commit ce7ee686c8d176013a8e3767c5a2d6a71cb9ba13 with merge 9db7021220dfbb6d33d8db1c1f1310a3bc4a1144...

@rust-highfive

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Dec 23, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 23, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 23, 2018

@bors try

@bors
Copy link
Contributor

bors commented Dec 23, 2018

⌛ Trying commit 844145f with merge d453555...

bors added a commit that referenced this pull request Dec 23, 2018
@bors
Auto merge of #57070 - Zoxc:ser-graph, r=<try>
d453555 
[do not merge] Revert "Convert SerializedDepGraph to be a struct-of-arrays"

Let's check if #49069 was actually helpful.

r? @michaelwoerister
@bors
Copy link
Contributor

bors commented Dec 23, 2018

☀️ Test successful - status-travis
State: approved= try=True

@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 23, 2018

@rust-timer build d453555

@rust-timer
Copy link
Collaborator

Success: Queued d453555 with parent 2d3e909, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit d453555

@Zoxc
Copy link
Contributor Author

Zoxc commented Dec 23, 2018

This doesn't seem to have much of an impact, but if it shifts computation to dep graph serialization it could still be beneficial to revert this for parallel queries.

@michaelwoerister
Copy link
Member

I think we got the information we needed (and #57061 is merged already). Thanks for collecting the numbers, @Zoxc!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@michaelwoerister michaelwoerister

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Zoxc @bors @rust-highfive @rust-timer @michaelwoerister